Skip to content

Conversation

@mbondyra
Copy link
Contributor

@mbondyra mbondyra commented Oct 15, 2024

With PR #187475 we introduced a bug, affecting the AI assistant's suggestions API when switching between different chart types (e.g., from bar to line chart). This feature relies on the switchVisualizationType method, which was changed to require a layerId. However, the suggestions API does not provide layerId, causing the chart type to not switch as expected.

Solution:
The bug can be resolved by modifying the logic in the switchVisualizationType method. We changed:

const dataLayer = state.layers.find((l) => l.layerId === layerId);

to:

const dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0];

This ensures that the suggestions API falls back to the first layer if no specific layerId is provided.

@mbondyra mbondyra added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// release_note:skip Skip the PR/issue when compiling release notes Feature:Lens backport:skip This PR does not require backporting labels Oct 15, 2024
@mbondyra mbondyra force-pushed the lens/fix_suggestions_api branch from 41a6664 to 6afce77 Compare October 15, 2024 12:24
@mbondyra mbondyra marked this pull request as ready for review October 15, 2024 12:25
@mbondyra mbondyra requested a review from a team as a code owner October 15, 2024 12:25
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbondyra thanx 🙇 LGTM!

@stratoula stratoula added backport:prev-minor and removed backport:skip This PR does not require backporting labels Oct 15, 2024
@mbondyra mbondyra force-pushed the lens/fix_suggestions_api branch from 6afce77 to 948b99e Compare October 15, 2024 12:42
@mbondyra mbondyra force-pushed the lens/fix_suggestions_api branch from 77abd20 to 0ac6c5c Compare October 15, 2024 13:04
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #18 / DataTableColumnHeader should render a correct token

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.5MB 1.5MB +17.0B

History

  • 💔 Build #242606 failed 77abd20fe28cbfdc57c32fa3940a3b6ee8516108

@mbondyra mbondyra merged commit e476220 into elastic:main Oct 15, 2024
@mbondyra mbondyra deleted the lens/fix_suggestions_api branch October 15, 2024 21:53
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11354960986

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 15, 2024
…#196295)

With [PR elastic#187475](https://github.com/elastic/kibana/pull/187475/files)
we introduced a bug, affecting the AI assistant's suggestions API when
switching between different chart types (e.g., from bar to line chart).
This feature relies on the switchVisualizationType method, which was
changed to require a `layerId`. However, the suggestions API does not
provide `layerId`, causing the chart type to not switch as expected.

Solution:
The bug can be resolved by modifying the logic in the
`switchVisualizationType` method. We changed:

```ts
const dataLayer = state.layers.find((l) => l.layerId === layerId);
```

to:

```ts
const dataLayer = state.layers.find((l) => l.layerId === layerId) ?? state.layers[0];
```

This ensures that the suggestions API falls back to the first layer if
no specific layerId is provided.

---------

Co-authored-by: Marco Vettorello <[email protected]>
(cherry picked from commit e476220)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 15, 2024
…196295) (#196448)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Lens] Fix switchVisualizationType to use it without layerId
(#196295)](#196295)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marta
Bondyra","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-15T21:53:00Z","message":"[Lens]
Fix switchVisualizationType to use it without layerId (#196295)\n\nWith
[PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe
introduced a bug, affecting the AI assistant's suggestions API
when\r\nswitching between different chart types (e.g., from bar to line
chart).\r\nThis feature relies on the switchVisualizationType method,
which was\r\nchanged to require a `layerId`. However, the suggestions
API does not\r\nprovide `layerId`, causing the chart type to not switch
as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying
the logic in the\r\n`switchVisualizationType` method. We
changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) =>
l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst
dataLayer = state.layers.find((l) => l.layerId === layerId) ??
state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API
falls back to the first layer if\r\nno specific layerId is
provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello
<[email protected]>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","Feature:Lens","v9.0.0","backport:prev-minor"],"title":"[Lens]
Fix switchVisualizationType to use it without
layerId","number":196295,"url":"https://github.com/elastic/kibana/pull/196295","mergeCommit":{"message":"[Lens]
Fix switchVisualizationType to use it without layerId (#196295)\n\nWith
[PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe
introduced a bug, affecting the AI assistant's suggestions API
when\r\nswitching between different chart types (e.g., from bar to line
chart).\r\nThis feature relies on the switchVisualizationType method,
which was\r\nchanged to require a `layerId`. However, the suggestions
API does not\r\nprovide `layerId`, causing the chart type to not switch
as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying
the logic in the\r\n`switchVisualizationType` method. We
changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) =>
l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst
dataLayer = state.layers.find((l) => l.layerId === layerId) ??
state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API
falls back to the first layer if\r\nno specific layerId is
provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello
<[email protected]>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196295","number":196295,"mergeCommit":{"message":"[Lens]
Fix switchVisualizationType to use it without layerId (#196295)\n\nWith
[PR #187475](https://github.com/elastic/kibana/pull/187475/files)\r\nwe
introduced a bug, affecting the AI assistant's suggestions API
when\r\nswitching between different chart types (e.g., from bar to line
chart).\r\nThis feature relies on the switchVisualizationType method,
which was\r\nchanged to require a `layerId`. However, the suggestions
API does not\r\nprovide `layerId`, causing the chart type to not switch
as expected.\r\n\r\nSolution:\r\nThe bug can be resolved by modifying
the logic in the\r\n`switchVisualizationType` method. We
changed:\r\n\r\n```ts\r\nconst dataLayer = state.layers.find((l) =>
l.layerId === layerId);\r\n```\r\n\r\nto:\r\n\r\n```ts\r\nconst
dataLayer = state.layers.find((l) => l.layerId === layerId) ??
state.layers[0];\r\n```\r\n\r\nThis ensures that the suggestions API
falls back to the first layer if\r\nno specific layerId is
provided.\r\n\r\n---------\r\n\r\nCo-authored-by: Marco Vettorello
<[email protected]>","sha":"e4762201fdd84f372c78bc2a159061e504b26e78"}}]}]
BACKPORT-->

Co-authored-by: Marta Bondyra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.16.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants